Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AP_RangeFinder: No specification of baud rate #14808

Closed

Conversation

muramura
Copy link
Contributor

@muramura muramura commented Jul 13, 2020

I found out that the UART->begin baud rate re-set doesn't work.
I looked at the source and misunderstood that it would be reconfigured.
I think it would be better to limit the buffer size specification to TX and RX.
I would set the baud rate setting to 0.

I would set the baud rate value to the SERIALx baud rate in the config parameter, if this feature is enabled. There is no need to set the It also eliminates the need to describe the WIKI baud rate.

#14796

@rmackay9
Copy link
Contributor

Does this mean that the "initial_baudrate" feature is not working? I think the purpose of this feature is to default the serial baud rate to the rate that the rangefinder supports. It should allow easier setup.

@peterbarker
Copy link
Contributor

Won't this break everybody's existing, working rangefinder setups?

@muramura
Copy link
Contributor Author

muramura commented Jul 14, 2020

@peterbarker san, @rmackay9 san. I checked the operation on my GY-US42v2 serial device.
I set the baud rate for the config parameter SERIALx to 115 and activated the FC.
I was able to confirm in the MAVLINK inspector that the GY-US42v2 would not give me any distance notification.
I believe the baud rate setting in the uart->begin method is not working.

I learned that if the baud rate specification in the uart->begin method is 0, it does not update the value of the internal variable baud rate.
I set the baud rate to 9 for this PR change and the config parameter SERIALx.
I was able to verify that the distance changes in the MAVLINK inspector.

ChibiOS UARTDriver.cpp:
if (b != 0) { ★ Variable b is the baud rate.
// clear buffers on baudrate change, but not on the console (which is usually USB)
if (_baudrate != b && hal.console != this) {
clear_buffers = true;
}
_baudrate = b;
}

@peterbarker
Copy link
Contributor

peterbarker commented Jul 16, 2020 via email

@muramura
Copy link
Contributor Author

muramura commented Jul 16, 2020

If uartdriver is correct, you should be able to use it normally. The wiki describes setting the baud rate of the serialx configuration parameter. The end user does not know that you are setting the baud rate in the range finder. Unless I wrote it on the wiki, I wouldn't know until I looked at the source. I looked at the source and expected it to be reset. However, since it has not been reconfigured, I think that returning to the wiki specifications will eliminate misunderstandings by developers.

I have confirmed that setting the baud rate to 0 will use the existing baud rate. If anything other than chibios occurs, I think there is a bug in the uartdriver.

@peterbarker
Copy link
Contributor

peterbarker commented Jul 17, 2020 via email

@Hwurzburg
Copy link
Collaborator

I dont have an issue if we wish to add that kind of wiki note...but its going to be many many places..... in general if we dont say you need to set something, then you dont need to set it....like in the telem protocols...many ignore the baud rate setting...but we tell people to use this, you need to set x,y,z...if w param is ignored, we dont say its ignored, its just not mentioned....many other examples...like serial options, rc options, often I2C addresses,etc...even baud rate on SERIAL0 when its USB...the list is almost endless...

@muramura
Copy link
Contributor Author

muramura commented Jul 19, 2020

I have been using the initial_baudrate of this driver on GY-US42v2 serial I learned that the method is not called.
The called initial_baudrate method gets the value of the config parameter and returns a There is.
I believe there is a problem with the access designation.

https://github.com/ArduPilot/ardupilot/blob/master/libraries/AP_RangeFinder/AP_RangeFinder_Backend_Serial.cpp#L41

@peterbarker
Copy link
Contributor

OK, this is definitely going ot break things.

If a specific driver needs to use 0 here for some reason it could by overriding initial_baudrate.

This is also relying on behaviour I'd like to remove from the serial manager - having it do the initial baud/buffer set up.

Closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants